-
Notifications
You must be signed in to change notification settings - Fork 8
Auction orchestration #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| ### Auction Flow | ||
| A named configuration that defines: | ||
| - Which providers participate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirming "providers" means wrappers?
| - Which providers participate | ||
| - Execution strategy (parallel, waterfall, etc.) | ||
| - Timeout settings | ||
| - Optional mediator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mediator = final ad-server?
| strategy = "parallel_mediation" | ||
| bidders = ["prebid", "aps"] | ||
| mediator = "gam" | ||
| timeout_ms = 2000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timeout meaning the time for PBS, APS etc to come back to TS right? This does not include any GAM RT's or think time?
|
|
||
| **Flow:** | ||
| 1. Prebid and APS run in parallel | ||
| 2. Both return their bids simultaneously |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better way to say is that "Both return their bids within a time window" as they'll never be simultaneous
|
|
||
| **Flow:** | ||
| 1. All bidders run in parallel | ||
| 2. Highest bid wins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be addressed elsewhere, but AFAIK APS doesn't return a clear text creative bid, so we'll need to understand what the decryption of the TAM value looks like
crates/common/src/auction/README.md
Outdated
|
|
||
| **Flow:** | ||
| 1. Try Prebid first | ||
| 2. If Prebid returns no bids, try APS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's discuss this on a call as it's not really a realistic scenario where PBS comes back with nothing and I don't know any pubs interested in waterfalls these days. We also need to make sure we are considering bid floors set by publishers here. Maybe I'm missing context?
92c19a7 to
39cb7dc
Compare
39cb7dc to
c82ffd3
Compare
aram356
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Good start
- Please address tests and formatting
- I would like to re-review
|
|
||
| let decoded = match STANDARD.decode(encoded) { | ||
| Ok(bytes) => bytes, | ||
| Err(_) => return 2.50, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔧 Add this as setting for default
🤔 We should logs errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually because eventually we're sending the encoded price to gam shouldn't I move any decode logic to the mock service anyway?
| timeout_ms = 800 | ||
|
|
||
| # Mock GAM Configuration (for testing) | ||
| [integrations.gam_mock] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔧 Why we need this because mocking is separated into different service
| /// | ||
| /// Initialized once on first access with the provided settings. | ||
| /// All providers are registered during initialization. | ||
| static GLOBAL_ORCHESTRATOR: OnceLock<AuctionOrchestrator> = OnceLock::new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛏️ Would be harder to test with these global singletons
Closes #139